Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes scrollBottom after view update #522

Closed
wants to merge 2 commits into from

Conversation

uniring
Copy link

@uniring uniring commented Feb 4, 2014

Triggers a hintResize before scrolling to bottom so it refreshes the contents size if you just updated the view contents.

I was doing a chat (whatsapp like) view and doing a $ionicScrollDelegate.scrollBottom() after fetching the contents. But it wasn’t scrolling well because the scroll view caches the height of the contents. This commit fixes that.

Triggers a hintResize before scrolling to bottom so it refreshes the
contents size if you just updated the view contents.

I was doing a chat (whatsapp like) view and doing a
$ionicScrollDelegate.scrollBottom() after fetching the contents. But it
wasn’t scrolling well because the scroll view caches the height of the
contents. This commit fixes that.
@mlynch
Copy link
Contributor

mlynch commented Feb 4, 2014

Hey @uniring, in the past I've recommended the user do this explicitly by calling $ionicScrollDelegate.resize() when the content changes.

Did you try that? I think it might make sense to put this one in here as well we just need to asses possible performance impact.

@uniring
Copy link
Author

uniring commented Feb 4, 2014

I've just tried that and it didn't work. This is my code:

    $scope.$watch('messages', function(val) {
        if (val.length > 0) {
            setTimeout(function() {
                $ionicScrollDelegate.resize();
                $ionicScrollDelegate.scrollBottom();
            }, 300);
        }
    }, true);

The 300ms timeout is to avoid scrolling while the push view animation is running (because sometimes I read the data from a cache and is too fast).

This will allow to select if you want to refresh the scroll content
size if you need it and improve performance if not.
@mlynch
Copy link
Contributor

mlynch commented Feb 4, 2014

@uniring, what if you defer it?

$ionicScrollDelegate.resize();
$timeout(function() {
  $ionicScrollDelegate.scrollBottom();
});

At any rate, we need to make a fix here, just want to see if that one worked for ya.

@uniring
Copy link
Author

uniring commented Feb 4, 2014

I've added a forceRefresh parameter to scrollBottom, this way you can select if you need a refresh or not.

Then my code looks like:

    $scope.$watch('messages', function(val) {
        if (val.length > 0) {
            setTimeout(function() {
                $ionicScrollDelegate.scrollBottom(true, true);
            }, 300);
        }
    }, true);

And works as expected. Another approach is to change the refresh function to work like the hintRefresh in the scrollView code, and using the $ionicScrollDelegate.resize() you mentioned, but I think this way is less complicated for the final user.

Any ideas?

@uniring
Copy link
Author

uniring commented Feb 4, 2014

Nice, defering it works well… Why I've don't tried it before ¬¬

Thanks! I'll close this pull request but keep in mind more users will try to implement this feature so hinting them in the docs will be a great idea :)

@uniring uniring closed this Feb 4, 2014
@uniring uniring reopened this Feb 4, 2014
@uniring
Copy link
Author

uniring commented Feb 4, 2014

Wow, I've missed the last line of your comment… It's not my day hehe. I've just reopened the pull request if you want it to merge, I don't know if this is the best approach, but it works. Just close it if you don't want it.

@ajoslin
Copy link
Contributor

ajoslin commented Feb 6, 2014

Added in nightly! Your use-case should work now without any extra code :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants